☁️ Nx Cloud Report

CI is running/has finished running commands for commit bfc79a7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

Sent with 💌 from NxCloud.

👋 Do you need anything from me to push this through?

Thanks for your contribution and for your patience @brendo ! Will try to merge/release today 🙏

@brendo FYI, we're seeing a perf issue on our unit tests and also a dramatically increased install size. We'll need to resolve those before we can merge.

The new version of @joshwooding/vite-plugin-react-docgen-typescript does grow in size, purely because glob does. I don't understand how that would lead to a 14 MB increase though

v0.3.1	v0.4.0

Oh that's really quite unfortunate. From poking at glob, it looks like jackspeak is used to power the bin and not for runtime operation. Would this mean it's tree-shaken away in typical apps?

Would this mean it's tree-shaken away in typical apps?

Good catch! Yes I'd expect that to treeshake. This is also evident by the size changes above, where the build size doesn't change, only the "install dependencies" size.

@ndelangen is anything stopping us from prebundling @joshwooding/vite-plugin-react-docgen-typescript, to circumven the transitive dependency of jackspeak (@joshwooding/vite-plugin-react-docgen-typescript -> glob@^10 > jackspeak.

@brendo you can try by just moving @joshwooding/vite-plugin-react-docgen-typescript to devDependencies, this should prebundle it instead and then we can see the numbers. Then we just need to test that it still works.

@ndelangen is anything stopping us from prebundling @joshwooding/vite-plugin-react-docgen-typescript, to circumven the transitive dependency of jackspeak (@joshwooding/vite-plugin-react-docgen-typescript -> glob@^10 > jackspeak.

It's not possible for me to say if it's possible without looking at the code of @joshwooding/vite-plugin-react-docgen-typescript.

If the runtime spawns a subprocess that's from it's own package; then it cannot.
If the runtime spawns a subprocess that's from another package, that package has to be added as a regular dep.
If the runtime tries to load a file relative to the runtime.
if the runtime tries to resolve something relative to the runtime.

Basically:
If @joshwooding/vite-plugin-react-docgen-typescript makes a runtime reference to itself or another package, that isn't a normal import bundlers can deal with, then you can't prebundle it.

That would include things such as:

• require.resolve('@joshwooding/vite-plugin-react-docgen-typescript/transformer')
• fs.readFile(__dirname + '/file.json')

Those code section, the bundler would leave untouched, but then their behavior most certainly not function as intended.

@brendo if prebundling doesn't work for some reason (it throws errors at runtime), alternatively you can try to just prebundle @joshwooding/vite-plugin-react-docgen-typescript by moving it to devDependencies, but not prebundle react-docgen-typescript where the magic happens, by adding that as an explicit dependencies.

So taking a closer look at the plugin, can we move it to a peerDependency? This package is conditional on if the user has configured the reactDocgen option to be react-docgen-typescript:

By default, this option appears to be react-docgen, which means the package won't be used:

It is reasonable to ask a user to install the package themselves instead of distributing it as part of @storybook/react-vite if they'd like to use the react-docgen-typescript option? Having that said, I noticed this option isn't documented so it'd be interesting to know how many users are specifying it. Nevermind, found via here and here.

I'm conflicted, ease of use vs package size.
@shilman thoughts on 👆?

(Moving it to a peer dep would be a breaking change in a major release, btw)

It can be closed because #29724 has been merged